simple DML translation #102

NathanQingyangXu · 2025-05-27T18:17:14Z

https://jira.mongodb.org/browse/HIBERNATE-46

The scope of the ticket is to unblock the following simple DML HQL mutation queries:

insert into Book (id, title) values (1, "War and Peace"), (2, "Crime and Punishmnet")
update Book set title = "War and Peace" where title = "War & Peace"
delete from Book where title = "War and Peace"

Translation is straightforward. The following new visitor methods would be implemented in this PR:

void visitDeleteStatement(DeleteStatement deleteStatement);
void visitUpdateStatement(UpdateStatement updateStatement);
void visitInsertStatement(InsertSelectStatement insertSelectStatement);

Both of the first two methods share a lot in common in the sense that they are both based on where clause, with various AST filters as the backbones, which has been finished in previous PR.

The scope of this PR is modest and only focuses on the following basic stuff:

no new AST filter will be introduced
when updating, only $set updating operator is supported (with the constraint that $set focuses on field path and value pair)

One caveat is previously our insertion command mongo ast only allows one document, to fully support HQL's feature, the mongo ast model was refactored to allow multiple documents, consistent with MQL's syntax as well. There is a separate ticket for this requirment: https://jira.mongodb.org/browse/HIBERNATE-44, so we can finish it in this PR additionally.

NathanQingyangXu · 2025-05-27T18:19:32Z

...grationTest/java/com/mongodb/hibernate/query/mutate/AbstractMutateQueryIntegrationTests.java

+        });
+    }
+
+    public static final class MutateTranslatorAwareDialect extends Dialect {


An important feature to test is we need to include the table names involved in DML queries into the result of SqlAstTranslator#getAffectedTableNames() which acts as auto-flush purpose in the upper level caller of the translator.

We might need to finish other features (like table joining) to test the auto-flush features in integration test; currently we could only test this important feature by some expedient.

It is related to auto-flushing criteria No..2 (see https://docs.jboss.org/hibernate/orm/6.6/userguide/html_single/Hibernate_User_Guide.html#flushing-auto):

prior to executing a JPQL/HQL query that overlaps with the queued entity actions

NathanQingyangXu · 2025-05-27T18:20:01Z

src/integrationTest/java/com/mongodb/hibernate/query/AbstractQueryIntegrationTests.java

@@ -36,17 +38,17 @@

 @SessionFactory(exportSchema = false)
 @ExtendWith(MongoExtension.class)
-abstract class AbstractSelectionQueryIntegrationTests implements SessionFactoryScopeAware, ServiceRegistryScopeAware {
+public abstract class AbstractQueryIntegrationTests implements SessionFactoryScopeAware, ServiceRegistryScopeAware {


this class is reusable not only for select query but also for mutate query, so it is renamed and moved to more general location. The Book class was changed accordingly to go together with it hand in hand.

NathanQingyangXu · 2025-05-27T18:26:06Z

src/main/java/com/mongodb/hibernate/internal/translate/MutationMqlTranslator.java

+            throw new FeatureNotSupportedException("Unsupported mutation statement type: "
+                    + mutationStatement.getClass().getName());
+        }
+    }


currently there is no fourth class of MutationStatement; the above last else clause is for future-proof purpose.

src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstInsertCommand.java

…t/command/AstInsertCommand.java Co-authored-by: Jeff Yemin <[email protected]>

NathanQingyangXu · 2025-05-27T19:44:14Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+        checkMutationStatement(insertStatement);
+        if (insertStatement.getConflictClause() != null) {
+            throw new FeatureNotSupportedException();
+        }


HQL has an interesting ON CONFLICT during insertion feature (see https://www.baeldung.com/hibernate-insert-query-on-conflict-clause). It is sort of similar to MongoDB's upsert feature but it seems there is a big difference between them:

HQL mainly focuses on conflict during insertion (w.r.t. unique constraint or fields combination); MongoDB's upsert focuses on conflict during updating (w.r.t. some query or filter,, e.g. comparison between a field and a value)

So I guess we can't support this HQL feature. HQL's update statement is simpler and provides no possibility to emulate MongoDB's upsert feature.

I think we could support the same semantics in MQL.

1. "on conflict do update:

What HQL does:
insert … on conflict do update - checks a unique index; if the key is taken it runs the UPDATE clause.

What MongoDB does:
update … { upsert:true } matches a filter; if no doc matches it inserts instead.

Despite starting from opposite directions (insert-first vs. update-first) they end up with the same two-way branch:

Case HQL MongoDB

No existing row/doc run regular INSERT insert via upsert

Conflict / match found run DO UPDATE … run UPDATE …

We could put the “excluded” columns in $set, and use $setOnInsert for values present only on insert.
Example command:

{ updateOne: { filter: { _id: 1 }, update: { // similar to DO UPDATE … SET title = excluded.title $set: { title: "Pride & Prejudice" }, // fields present only on first insert $setOnInsert: { title: "Pride & Prejudice" outOfStock: false, publishYear: 1813, isbn13: NumberLong("9780141439518"), discount: 0.2, price: Decimal128("23.55") } }, upsert: true } },

2. "on conflict do nothing":

We could use ordered: false for insert operations to process all inserts and ignore any duplication errors reported via writeErrors. For this, though, we’d need to be aware of the HQL context, if possible.

In any case, I suggest we handle this as a separate ticket, so as not to block this PR and to keep the review focused. Let me know your thoughts.

Definitively it deserves a ticket. I think all of the following HQL criteria needs to be satisfied to allow for MQL translation:

ON CONFLICT is specified

ON CONSTRAINT uniqueIndexName is not used as the criteria (MQL's filter can't be based on unique index name, correct?)

CONFLICT ACTION is not NOTHING

so it seems the ticket is an interesting tech one but could be used very rarely in reality; native query seems more flixible and powerful.

I created a ticket here: https://jira.mongodb.org/browse/HIBERNATE-94. Let me know when you have any further suggestion.

NathanQingyangXu · 2025-05-27T20:25:22Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+        }
+        if (insertStatement.getSourceSelectStatement() != null) {
+            throw new FeatureNotSupportedException();
+        }


HQL has two flavours of insertion statement:

insert into Book (id, title) values (1, "War and Peace"), (2, "Crime and Punishment")

insert into Book (id, title) select c.identifier, c.name from Catalog as c

This PR will focus on the first variant and throw exception for the latter.

Do we have a ticket for the latter? If not, should we create one in the backlog to track it?

We don't have a ticket. From my understanding, there is no MQL counterpart so we won't support it in the first place.

We have $out and $merge. Neither is a direct counterpart to insert into ... select ..., despite $out being advertized as the counterpart (it "conveniently" for applications deletes the target collection, which SQL never does), but maybe there is a way to implement insert into ... select ... in our extension in a way that at least partially mimics the expected behavior.

I did consider the possibility to emulate insert into ... select ... using aggregate pipeline, but I am not sure whether it is worthwhile; one of the reasons is our user could easily achieve the same goal by breaking down the process to two separate steps programmatically (first grabbing all the values using some selection or native query and then using the normal insertion mutation or native query) and it is much more flexible.
If you persist, I can create a ticket of lowest priority.

NathanQingyangXu · 2025-05-27T20:28:30Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+            documents.add(new AstDocument(astElements));
+        }
+
+        astVisitorValueHolder.yield(COLLECTION_MUTATION, new AstInsertCommand(collection, documents));


there are 3 assertion usages above as follows:

insertion field list is not empty

insertion values list is not empty

field list length is the same as values length

The above constraints were either enforced from HQL grammar level or semantic checking phase in Hibernate (a prior step when the final SQL AST translation step is reached), so we can rest assured these constraints are enforced already.

similarly, various assertions are used in updating case as well. Common constraints have been carefully centralized into checkMutationStatementSupportability() method. Only one checking is required to ensure the insertion field value or udpating field value expressions should be either valueExpression (literal value or parameter expressions).

…he old selection queries testing caeses

# Conflicts: # src/integrationTest/java/com/mongodb/hibernate/query/select/AbstractSelectionQueryIntegrationTests.java # src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

vbabanin · 2025-07-09T05:44:41Z

src/integrationTest/java/com/mongodb/hibernate/query/AbstractQueryIntegrationTests.java

+                    return delegate.getSqlAstTranslatorFactory().buildModelMutationTranslator(mutation, sessionFactory);
+                }
+
+                private <T extends JdbcOperation> SqlAstTranslator<T> createCapturingTranslator(


[nit] Let’s make the name a bit more descriptive here to show the intent, something like createTranslateResultCapturingAstTranslator.

src/main/java/com/mongodb/hibernate/internal/translate/ModelMutationMqlTranslator.java

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

src/integrationTest/java/com/mongodb/hibernate/query/mutation/InsertionIntegrationTests.java

…tMqlTranslator.java Co-authored-by: Viacheslav Babanin <[email protected]>

…ractMqlTranslator

…InsertionIntegrationTests.java Co-authored-by: Viacheslav Babanin <[email protected]>

…nt() as well

…se 'is not supported' as opposed to 'not supported'

NathanQingyangXu · 2025-07-18T19:26:22Z

@stIncMale , would appreciate your taking a secondary review for @vbabanin had approved it for a while. Thanks.

stIncMale · 2025-07-22T06:50:58Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+    private static void checkCteContainerSupportability(CteContainer cteContainer) {
+        if (!cteContainer.getCteStatements().isEmpty()
+                || !cteContainer.getCteObjects().isEmpty()) {
+            throw new FeatureNotSupportedException("CTE is ");


The error message is incomplete.

Good catch, thanks!

stIncMale · 2025-07-22T15:16:13Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+        }
+        var root = fromClause.getRoots().get(0);
+        if (root.hasRealJoins()) {
+            throw new FeatureNotSupportedException("TODO-HIBERNATE-65 https://jira.mongodb.org/browse/HIBERNATE-65");


https://jira.mongodb.org/browse/HIBERNATE-65 is linked here in the TODO comment, but there is no note in the ticket description about resolving such TODOs. We must add it similarly to how it is done in https://jira.mongodb.org/browse/HIBERNATE-44, for example.

yeah, we should have. Done.

stIncMale · 2025-07-22T20:59:26Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+    private String addToAffectedCollections(NamedTableReference tableRef) {
+        var collection = tableRef.getTableExpression();
+        affectedTableNames.add(collection);
+        return collection;


The field containing the list is named affectedTableNames, but the method adding to the list is named addToAffectedCollections. Let's rename the method to addToAffectedTableNames.

My original intention is to map closer to our domain model at least on method level (field name is lower-level and mapped to Hibernate's particular); but I changed as you suggested.

stIncMale · 2025-07-22T22:02:45Z

src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

+        }
+        if (insertStatement.getSourceSelectStatement() != null) {
+            throw new FeatureNotSupportedException();
+        }


We have $out and $merge. Neither is a direct counterpart to insert into ... select ..., despite $out being advertized as the counterpart (it "conveniently" for applications deletes the target collection, which SQL never does), but maybe there is a way to implement insert into ... select ... in our extension in a way that at least partially mimics the expected behavior.

stIncMale · 2025-07-23T00:56:56Z

...egrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java

@@ -60,6 +58,12 @@ private static List<Book> getBooksByIds(int... ids) {
                .toList();
    }

+    @BeforeEach
+    protected void beforeEach() {


Why was this method moved from the top of the class? We previously moved all such methods to the top specifically to make the code consistent, instead of having them scattered in different places of the class.

I didn't notice such convention and I tried to be consistent with LimitOffsetFetchClauseIntegrationTests; relocate @BeforeEach in both classes after field declaration.

stIncMale · 2025-07-23T01:39:43Z

src/integrationTest/java/com/mongodb/hibernate/query/AbstractQueryIntegrationTests.java

+    @Override
+    public void injectSessionFactoryScope(SessionFactoryScope sessionFactoryScope) {
+        this.sessionFactoryScope = sessionFactoryScope;
+    }
+
+    @Override
+    public void injectServiceRegistryScope(ServiceRegistryScope serviceRegistryScope) {
+        this.testCommandListener = serviceRegistryScope.getRegistry().requireService(TestCommandListener.class);
+    }


Let's move these methods to the top of the class, right after the field declarations. That way they will be placed consistently with the intentional placement in the other classes.

ok, didn't notice we have such convention.

stIncMale · 2025-07-23T01:56:28Z

...st/java/com/mongodb/hibernate/query/select/BooleanExpressionWhereClauseIntegrationTests.java


    private Book bookOutOfStock;
    private Book bookInStock;

    @BeforeEach
-    void beforeEach() {
+    protected void beforeEach() {


What is the reason for making this method protected?

this is a legacy minor issue; tbh I don't recollect the reason. Removed.

It is not a legacy issue; I overlooked it during refactoring; I tried to move some common beforeEach() to the AbstractQueryIntegrationTests parent class and then protected might be needed; later on I aborted the experimenting and forgot to remove it.

stIncMale · 2025-07-23T01:58:28Z

...egrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java

@@ -60,6 +58,12 @@ private static List<Book> getBooksByIds(int... ids) {
                .toList();
    }

+    @BeforeEach
+    protected void beforeEach() {


What is the reason for making this method protected? As far as I can tell, it should be package-access.

Suggested change

protected void beforeEach() {

void beforeEach() {

src/integrationTest/java/com/mongodb/hibernate/query/mutation/UpdatingIntegrationTests.java

src/integrationTest/java/com/mongodb/hibernate/query/mutation/InsertionIntegrationTests.java

stIncMale · 2025-07-23T02:27:58Z

The last reviewed commit is ea75d2f.

…ueryIntegrationTests.java Co-authored-by: Valentin Kovalenko <[email protected]>

…UpdatingIntegrationTests.java Co-authored-by: Valentin Kovalenko <[email protected]>

NathanQingyangXu marked this pull request as draft May 27, 2025 18:17

NathanQingyangXu commented May 27, 2025

View reviewed changes

NathanQingyangXu force-pushed the HIBERNATE-46 branch from 25d8fb7 to 8e2cee5 Compare May 27, 2025 18:36

jyemin reviewed May 27, 2025

View reviewed changes

src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstInsertCommand.java Outdated Show resolved Hide resolved

simple DML translation

55ab573

NathanQingyangXu force-pushed the HIBERNATE-46 branch from 8e2cee5 to 55ab573 Compare May 27, 2025 18:49

Update src/main/java/com/mongodb/hibernate/internal/translate/mongoas…

c4deaf9

…t/command/AstInsertCommand.java Co-authored-by: Jeff Yemin <[email protected]>

NathanQingyangXu commented May 27, 2025

View reviewed changes

NathanQingyangXu added 4 commits May 27, 2025 16:30

minor improvement to AbstractMqlTranslator#visitInsertStatement()

d71f568

minor improvements to AbstractMqlTranslator

1655c57

improvements to naming

163fb45

Merge branch 'main' into HIBERNATE-46

f8edea4

NathanQingyangXu force-pushed the HIBERNATE-46 branch from 42d2b77 to 5c7a58d Compare May 28, 2025 19:39

NathanQingyangXu marked this pull request as ready for review May 29, 2025 13:10

NathanQingyangXu requested review from vbabanin, stIncMale and jyemin May 29, 2025 13:10

NathanQingyangXu marked this pull request as draft May 29, 2025 14:21

NathanQingyangXu removed request for vbabanin and stIncMale May 29, 2025 14:21

NathanQingyangXu requested review from vbabanin and stIncMale June 10, 2025 20:30

NathanQingyangXu force-pushed the HIBERNATE-46 branch 2 times, most recently from 0cc0a35 to 4d24c09 Compare June 11, 2025 18:26

some improvements

6558ab1

NathanQingyangXu force-pushed the HIBERNATE-46 branch from 4d24c09 to 6558ab1 Compare June 11, 2025 18:29

NathanQingyangXu requested a review from vbabanin June 25, 2025 13:58

NathanQingyangXu added 3 commits June 30, 2025 09:23

add affectedTableNames generic validation covering both new DML and t…

6eb6578

…he old selection queries testing caeses

Merge branch 'main' into HIBERNATE-46

90f8509

resolve conflict with latest main branch

9c35cf5

NathanQingyangXu marked this pull request as ready for review July 2, 2025 22:09

Merge branch 'main' into HIBERNATE-46

f1cf544

# Conflicts: # src/integrationTest/java/com/mongodb/hibernate/query/select/AbstractSelectionQueryIntegrationTests.java # src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java

vbabanin reviewed Jul 9, 2025

View reviewed changes

NathanQingyangXu and others added 6 commits July 9, 2025 16:25

Update src/main/java/com/mongodb/hibernate/internal/translate/Abstrac…

3466624

…tMqlTranslator.java Co-authored-by: Viacheslav Babanin <[email protected]>

Update src/main/java/com/mongodb/hibernate/internal/translate/Abstrac…

94a5dfe

…tMqlTranslator.java Co-authored-by: Viacheslav Babanin <[email protected]>

improve by removing the usage of CollectionAndFilter record from Abst…

aed1a1b

…ractMqlTranslator

Update src/integrationTest/java/com/mongodb/hibernate/query/mutation/…

4737f83

…InsertionIntegrationTests.java Co-authored-by: Viacheslav Babanin <[email protected]>

reuse the new addToAffectedCollections() method in visitInsertStateme…

a1095e8

…nt() as well

add JIRA reference for insert conflict message; improve verbiage to u…

56296e6

…se 'is not supported' as opposed to 'not supported'

vbabanin previously approved these changes Jul 9, 2025

View reviewed changes

fix broken testing case due to the last commit

ea75d2f

NathanQingyangXu dismissed vbabanin’s stale review via ea75d2f July 10, 2025 14:00

NathanQingyangXu requested a review from vbabanin July 10, 2025 14:01

stIncMale requested changes Jul 23, 2025

View reviewed changes

NathanQingyangXu and others added 2 commits July 23, 2025 10:12

resolve code review comments

5923bb6

Update src/integrationTest/java/com/mongodb/hibernate/query/AbstractQ…

1172280

…ueryIntegrationTests.java Co-authored-by: Valentin Kovalenko <[email protected]>

NathanQingyangXu requested a review from a team as a code owner July 23, 2025 14:14

NathanQingyangXu and others added 6 commits July 23, 2025 10:15

Merge remote-tracking branch 'origin/HIBERNATE-46' into HIBERNATE-46

d9bfa71

Update src/integrationTest/java/com/mongodb/hibernate/query/mutation/…

05f1b4d

…UpdatingIntegrationTests.java Co-authored-by: Valentin Kovalenko <[email protected]>

minor changes per review comments

7e875eb

Merge remote-tracking branch 'origin/HIBERNATE-46' into HIBERNATE-46

7b31d84

minor changes per review comments further

2d7ec77

relocate @beforeeach right after fields declaration

d128182

NathanQingyangXu requested a review from stIncMale July 23, 2025 14:30

Case	HQL	MongoDB
No existing row/doc	run regular INSERT	insert via upsert
Conflict / match found	run DO UPDATE …	run UPDATE …

simple DML translation #102

Are you sure you want to change the base?

simple DML translation #102

Uh oh!

Conversation

NathanQingyangXu commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

1. "on conflict do update:

2. "on conflict do nothing":

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vbabanin Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NathanQingyangXu commented Jul 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu commented May 27, 2025 •

edited

Loading

NathanQingyangXu Jun 11, 2025 •

edited

Loading

vbabanin Jun 24, 2025 •

edited

Loading

NathanQingyangXu Jul 23, 2025 •

edited

Loading

vbabanin Jul 9, 2025 •

edited

Loading